-
-
Notifications
You must be signed in to change notification settings - Fork 437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mage_Log - DOC block update #767
Conversation
sreichel
commented
Jun 23, 2019
- doc blocks added/fixed
- PSR2 fixes (whitespaces, linebreaks, ...)
- doc blocks added/fixed - PSR2 fixes (whitespaces, linebreaks, ...)
@@ -58,6 +58,9 @@ protected function _construct() | |||
$this->_init('log/log'); | |||
} | |||
|
|||
/** | |||
* @return float|int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why float? the config description is "Save Expired Items Days" and default value is "180"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mage::getStoreConfig(self::XML_LOG_CLEAN_DAYS)
returns a string
... multiplied its float.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, so we need a casting to int here.
I propose we either skip this docbloc change, and open a new PR fixing the docblock and adding cast.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
created an issue for this change #949
@@ -158,6 +187,9 @@ public function getUrl() | |||
return $url; | |||
} | |||
|
|||
/** | |||
* @return mixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why mixed? I wold expect rather int
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used mixed only in a very few cases, where multiple return types are possble. Mixed/object are on todo list as mentioned in #784.
@@ -166,6 +198,9 @@ public function getFirstVisitAt() | |||
return $this->getData('first_visit_at'); | |||
} | |||
|
|||
/** | |||
* @return mixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above.
@@ -58,6 +58,9 @@ protected function _construct() | |||
$this->_init('log/log'); | |||
} | |||
|
|||
/** | |||
* @return float|int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
created an issue for this change #949
thanks @sreichel ! 🎉 |
I am happily surprised :) Didnt check ... if this is discussed in discord plmk. |